Skip to content

Improve cmake#100

Open
MhmRhm wants to merge 1 commit into
kosma:masterfrom
MhmRhm:improve-cmake
Open

Improve cmake#100
MhmRhm wants to merge 1 commit into
kosma:masterfrom
MhmRhm:improve-cmake

Conversation

@MhmRhm

@MhmRhm MhmRhm commented Jun 9, 2026

Copy link
Copy Markdown

Here is a clean, modern, FetchContent-friendly CMake rewrite of the project. It is structured so that:

  • works as standalone project
  • works via FetchContent
  • supports install() + find_package()
  • keeps examples/tests optional
  • avoids global flag pollution
  • uses proper target-based CMake

How users consume it (FetchContent)

include(FetchContent)

FetchContent_Declare(
    minmea
    GIT_REPOSITORY https://github.com/your/minmea.git
    GIT_TAG master
)

FetchContent_MakeAvailable(minmea)

target_link_libraries(my_app PRIVATE minmea::minmea)

Or with find_package (after install)

find_package(minmea REQUIRED)

target_link_libraries(my_app PRIVATE minmea::minmea)

What is improved vs original

✔ Fully target-based

No global CMAKE_C_FLAGS

✔ Safe for embedding

Works inside other projects without side effects

✔ FetchContent compatible

No assumptions about install step

✔ Proper namespace

minmea::minmea

✔ Clean install/export system

Supports find_package

✔ Optional components

  • examples
  • tests
  • install toggle

✔ Modern CMake practices

  • generator expressions
  • interface include dirs
  • alias target

@kosma kosma added merge queue Scheduled for merging when someone has time to look at it and review. requires thinking This is a significant architectural decision that can't be done quickly. labels Jul 15, 2026
@kosma

kosma commented Jul 15, 2026

Copy link
Copy Markdown
Owner

@MhmRhm Can you have a look at #85 and tell me if it's needed or necessary to move minmea.h to include/? I kinda enjoy having the file in the root of the repository, and my knowledge of CMake is exactly zero, so I would love if someone with more knowledge would share their thoughts.

@MhmRhm

MhmRhm commented Jul 15, 2026

Copy link
Copy Markdown
Author

To the best of my knowledge, the main reason is that this is the conventional layout for libraries intended to be consumed by other projects.

Instead of exposing headers directly, it is generally recommended to organize them so that consumers can include them like this:

#include <project_name/header_name.h>

This helps avoid collisions if multiple libraries happen to provide a header with the same name. A generic:

#include <minmea.h>

is more likely to conflict than:

#include <minmea/minmea.h>

This convention also works well with package managers like vcpkg and CMake's FetchContent. From the consumer's perspective, these libraries are used as if they were installed system-wide, so following a standard include layout makes integration more predictable and consistent.

I first came across this recommendation in the first edition of Modern CMake for C++ by Rafał Świdziński. On page 96, it suggests structuring a library like this:

lib3/
├── include/
│   └── lib3/
│       └── public.h
├── test/
├── CMakeLists.txt
└── lib3.cpp

When the include/ directory is installed, the internal structure is preserved, so consumers can include the header as:

#include <lib3/public.h>

rather than:

#include <public.h>

So I think moving minmea.h under include/minmea/ would align the project with common CMake and C++ library conventions, especially if the library is intended to be distributed through package managers.

That said, I don't think this is strictly required if the project is intentionally kept as a single-header library; it is more about making the library easier to consume as it grows.

@MhmRhm

MhmRhm commented Jul 15, 2026

Copy link
Copy Markdown
Author

Also dear @kosma will appreciate it if you take a look at the pull request in vcpkg for this library:

microsoft/vcpkg#52281 (comment)

The following is a minimal example that fetches minmea from vcpkg:

CMakeLists.txt
main.cpp
CMakePresets.json
vcpkg.json

You can put all these in an empty directory and run:

cmake --preset linux-default-debug
cmake --build --preset linux-default-debug

Just make sure vcpkg is installed in ~/.vcpkg instead of ~/vcpkg. That's where my CMakePresets.json expect it to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge queue Scheduled for merging when someone has time to look at it and review. requires thinking This is a significant architectural decision that can't be done quickly.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants